Add CMake target for every remaining library#263
Conversation
This should, in principle, enable a full Au installation via CMake! I took a more expansive view of which libraries are "for public consumption" relative to bazel. The old division was based on a strong expectation that users would always want to include one or at most a very few files, before we fully appreciated the benefits of file-by-file inclusion for build speed. I'll bring bazel in line in a follow up PR. In making these CMake targets, I discovered two small irregularities in their bazel counterparts --- namely, a missing dependency, and a bizarre `copts` line that I think came from a stray default vim snippet. I fixed them in this same PR. Helps #215. In order to resolve it fully, we'll need to tidy up our documentation posture w.r.t. CMake.
geoffviola
left a comment
There was a problem hiding this comment.
Great!
I tested it with this command
docker run --rm \
ubuntu@sha256:2e863c44b718727c860746568e1d54afd13b2fa71b160f5cd9058fc436217b30 \
/bin/bash -c \
'apt update && apt install -y clang cmake git && git clone -b chiphogg/cmake-other-libs#215 https://github.com/aurora-opensource/au.git && cd au && cmake -S . -B cmake/build -DCMAKE_VERIFY_INTERFACE_HEADER_SETS=TRUE && cmake --build cmake/build -j $(proc) --target all all_verify_interface_header_sets && ctest -j $(nproc) --test-dir cmake/build'
As a side note, omitting test from cmake --build cmake/build -j $(proc) --target all all_verify_interface_header_sets means that the tests don't run. For whatever reason, -j works on the build step, but not the test step. Running ctest alone allows the tests to run in parallel.
au/CMakeLists.txt
Outdated
| quantity_point | ||
| unit_of_measure | ||
| unit_symbol | ||
| GTEST_SRCS |
There was a problem hiding this comment.
| GTEST_SRCS | |
| GTEST_SRCS | |
| prefix_test.cc | |
| GTEST_EXTRA_DEPS |
au/CMakeLists.txt
Outdated
| # | ||
|
|
||
| header_only_library( | ||
| NAME au |
There was a problem hiding this comment.
I found it a little weird that the indentation is 3 spaces. It's not actually a problem, but I'm more familiar with 2 spaces. At some point, we may want to use a formatter like https://github.com/cheshirekow/cmake_format.
There was a problem hiding this comment.
For some reason, 3 spaces was what my editor chose. Super weird! Fixed by switching to 2.
|
Did we want to support no_wconversion_test.cc in CMake? |
Sorry, I should have mentioned: I decided to skip that one, as well as |
Not sure why it was 3 before, but that was a weird value.
This should, in principle, enable a full Au installation via CMake!
I took a more expansive view of which libraries are "for public
consumption" relative to bazel. The old division was based on a strong
expectation that users would always want to include one or at most a
very few files, before we fully appreciated the benefits of file-by-file
inclusion for build speed. I'll bring bazel in line in a follow up PR.
In making these CMake targets, I discovered two small irregularities in
their bazel counterparts --- namely, a missing dependency, and a bizarre
coptsline that I think came from a stray default vim snippet. Ifixed them in this same PR.
Helps #215. In order to resolve it fully, we'll need to tidy up our
documentation posture w.r.t. CMake.